feat(sdk-coin-dot): add MPCv2 support to recoverConsolidations - #9486

WCI-1236

AI-generated review — auto-generated by Claude Code (wallet-platform:review-pr). Please use your own judgment before acting on any findings.

Phase 1 — Summary

This PR adds MPCv2 support to Dot.recoverConsolidations() by hoisting the getEddsaSigningMaterial call out of the per-index recover() loop and threading the resolved material in via a new precomputedMaterial?: EddsaSigningMaterial parameter on recover(). It also adds the MPCv2 signed path to standalone recover() itself (previously only MPCv1 and unsigned-sweep existed). Two files changed: modules/sdk-coin-dot/src/dot.ts (+101/−34) and modules/sdk-coin-dot/test/unit/dot.ts (+145/−100).

flowchart TD
    A[recoverConsolidations] -->|walletPassphrase && userKey| B[getEddsaSigningMaterial — called ONCE]
    A -->|no passphrase| C[signingMaterial = undefined]
    B --> D{loop over indexes}
    C --> D
    D -->|each index| E[recover(recoverParams, signingMaterial)]
    E -->|precomputedMaterial != null| F[use precomputedMaterial — skip per-call detection]
    E -->|precomputedMaterial == null| G[getEddsaSigningMaterial — standalone path]
    F --> H[addRecoverySignature / signEddsaMpcV2RecoveryTx]
    G --> H

Phase 2 — Ticket Alignment

Ticket: WCI-1236

PR title: feat(sdk-coin-dot): add MPCv2 support to recoverConsolidations — aligns well. The implementation matches the stated goal (detect-once, thread material, fix base address derivation for MPCv2). One minor discrepancy: the PR body describes a multisigTypeVersion: 'MPCv2' option on DotRecoveryOptions, but the actual implementation uses the cleaner precomputedMaterial second parameter instead. The end result is equivalent or better, but the body/spec diverged from what landed.

Phase 3 — Index Analysis

No new database queries introduced.

Phase 4 — Type Safety

Two double-cast patterns in test code:

basecoin as unknown as { getEddsaSigningMaterial: unknown }

This appears twice (in the recover() and recoverConsolidations() test suites). It's a common sinon pattern for spying on private methods; no production code is affected. No as any, @ts-ignore, or unsafe casts found in dot.ts.

consolidationTransactions: any[] on line ~493 of dot.ts is pre-existing (unchanged by this PR).

Phase 5 — MPCv2 Correctness & Test Quality

Correctness

• ✅ recoverConsolidations() detects signing material ONCE before the loop — signingMaterial is resolved before the for loop and threaded into every recover() call.
• ✅ Detection guarded on params.walletPassphrase && params.userKey — exact guard in diff; unsigned sweeps correctly pass undefined.
• ✅ recover() short-circuits with precomputedMaterial ?? before calling getEddsaSigningMaterial — correct null-coalescing pattern.
• ✅ assert() guards remain in recover() for the standalone path — assert(params.backupKey, ...) and assert(params.walletPassphrase, ...) are untouched.

Test Quality

• ✅ Test asserts getEddsaSigningMaterial called exactly once across a multi-index scan — sandBox.assert.calledOnce(getEddsaMaterialSpy) in "should call getEddsaSigningMaterial exactly once across all recover() iterations".
• ✅ MPCv2 signed consolidation test verifies correct base address via tx1.toJson().to === baseAddr where baseAddr is derived from mpcV2CommonKeyChain via MPCv2-correct derivation (mpc.deriveUnhardened(mpcV2CommonKeyChain, 'm/0')).
• ✅ MPCv1 regression test for consolidations — "should leave MPCv1 base address derivation and signing unchanged (regression)" covers existing WRW keycard path and checks destination address.
• ✅ Unsigned sweep consolidation test — "should leave the unsigned (no passphrase) cold path unchanged when keycard is MPCv2".
• ✅ Spy/stub assertions are specific — calledOnce, notCalled used consistently; no bare truthy checks.

Consistency vs related PRs

• ✅ Same detection-once pattern as ADA consolidations (feat(sdk-coin-ada): add MPCv2 support to recoverConsolidations #9497) — signingMaterial hoisted before loop, passed as second arg.
• ⚠️ Minor: the two consolidation tests ("should build MPCv2 signed consolidation recoveries..." and "should call getEddsaSigningMaterial exactly once across all recover() iterations") overlap significantly — both spy the same method, run the same scan range (1–4), and assert calledOnce. Consider collapsing into one or ensuring each tests a distinct concern.

Summary

No blocking issues. The detection-once pattern is implemented correctly and the test suite covers the main paths (MPCv2 signed, MPCv1 regression, unsigned cold path). The small points worth a follow-up:

1. PR body/spec drift — multisigTypeVersion option mentioned in the body was not implemented; precomputedMaterial parameter was used instead (fine, but body should be updated or noted for reviewers).
2. Duplicate test coverage — two consolidation tests assert calledOnce on getEddsaSigningMaterial for the same invocation; deduplicate or differentiate.
3. Double cast in tests (as unknown as { getEddsaSigningMaterial: unknown }) — minor, acceptable for private-method spying, but worth a comment explaining why.

Generated by [wallet-platform:review-pr] via Claude Code